[Fix-17817][Master]Add workflow timeout event and handle by njnu-seafish · Pull Request #18063 · apache/dolphinscheduler

njnu-seafish · 2026-03-13T03:05:46Z

Was this PR generated or assisted by AI?

NO

Purpose of the pull request

This bug involves a critical feature and has been stagnant for several months. Since I actually resolved the task timeout and workflow timeout issue quite some time ago, I would like to finalize the process and get it closed.

Brief change log

Add workflow timeout event and handle

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

...inscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java

+@AllArgsConstructor(access = AccessLevel.PRIVATE)
+public class WorkflowTimeoutLifecycleEvent extends AbstractWorkflowLifecycleLifecycleEvent {
+
+    private IWorkflowExecutionRunnable workflowExecutionRunnable;


Copilot

Pull request overview

This PR addresses issue #17817 by introducing a workflow-level timeout lifecycle event/handler so that configured workflow timeout alerts are actually emitted by the master engine.

Changes:

Add a new WorkflowTimeoutLifecycleEvent + WorkflowTimeoutLifecycleEventHandler, and wire publishing from WorkflowStartLifecycleEventHandler.
Add TIMEOUT to WorkflowLifecycleEventType to support routing the new event.
Add service/DAO plumbing for sending workflow-timeout alerts and refactor alert persistence helper naming; tighten task-timeout validation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/alert/WorkflowAlertManager.java	Adds `sendWorkflowTimeoutAlert` entrypoint to reach DAO alert creation.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandler.java	New handler to send workflow timeout alerts when the timeout event fires.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/handler/WorkflowStartLifecycleEventHandler.java	Publishes workflow timeout event at workflow start (if timeout configured).
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java	New delay event representing workflow timeout.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/WorkflowLifecycleEventType.java	Adds `TIMEOUT` event type.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/task/lifecycle/event/TaskTimeoutLifecycleEvent.java	Tightens validation to require timeout minutes > 0.
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/AlertDao.java	Adds validation for workflow timeout alert inputs and consolidates timeout-alert saving helper.
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AlertType.java	Updates comment to reflect actual enum values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

...inscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java

+        long delayTime = System.currentTimeMillis() - workflowInstance.getStartTime().getTime()
+                + TimeUnit.MINUTES.toMillis(timeout);


njnu-seafish · 2026-03-20T03:38:00Z

...inscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java

+
+    public static WorkflowTimeoutLifecycleEvent of(IWorkflowExecutionRunnable workflowExecutionRunnable) {
+        final WorkflowInstance workflowInstance = workflowExecutionRunnable.getWorkflowInstance();
+        checkState(workflowInstance != null, "The workflow instance must be initialized before retrying.");


njnu-seafish · 2026-03-20T03:37:24Z

...uler/server/master/engine/workflow/lifecycle/handler/WorkflowStartLifecycleEventHandler.java

+    private void workflowTimeoutMonitor(final IWorkflowExecutionRunnable workflowExecutionRunnable) {
+        final WorkflowInstance workflowInstance = workflowExecutionRunnable.getWorkflowInstance();
+        if (workflowInstance.getTimeout() <= 0) {
+            log.debug("The workflow {} timeout {} is invalided, so the timeout monitor will not be started.",


ruanwenjun

Please review your PR first, too many of these comments have been raised repeatedly in previous PRs.

ruanwenjun · 2026-03-18T13:18:23Z

...inscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java

+        final int timeout = workflowInstance.getTimeout();
+        checkState(timeout > 0, "The workflow timeout: %s must > 0 minutes", timeout);
+
+        long delayTime = System.currentTimeMillis() - workflowInstance.getStartTime().getTime()


When the workflow instance is rerun the startTime will not change. You should use restartTime .

When the workflow instance is rerun the startTime will not change. You should use restartTime .

good

ruanwenjun · 2026-03-18T13:20:06Z

...uler/server/master/engine/workflow/lifecycle/handler/WorkflowStartLifecycleEventHandler.java

+    private void workflowTimeoutMonitor(final IWorkflowExecutionRunnable workflowExecutionRunnable) {
+        final WorkflowInstance workflowInstance = workflowExecutionRunnable.getWorkflowInstance();
+        if (workflowInstance.getTimeout() <= 0) {
+            log.debug("The workflow {} timeout {} is invalided, so the timeout monitor will not be started.",


The log is incorrect.

The log is incorrect.

fix it

ruanwenjun

You should add IT case.

ruanwenjun · 2026-03-18T13:26:11Z

...che/dolphinscheduler/server/master/engine/workflow/lifecycle/WorkflowLifecycleEventType.java

     * Pause the workflow instance
     */
    PAUSE,
+


Please don't change the unrelated code.

Please don't change the unrelated code.

ok

njnu-seafish · 2026-03-20T08:02:49Z

You should add IT case.

added

SbloodyS · 2026-03-25T02:25:11Z

dolphinscheduler-master/src/test/resources/it/start/workflow_with_workflow_timeout_alert.yaml

+    postTaskCode: 1
+    postTaskVersion: 1
+    createTime: 2024-08-12 00:00:00
+    updateTime: 2024-08-12 00:00:00


Suggested change

updateTime: 2024-08-12 00:00:00

updateTime: 2024-08-12 00:00:00

We should leave an blank line at the end of each file.

We should leave an blank line at the end of each file.

ok

Copilot

Pull request overview

Adds workflow-timeout alerting to the Master workflow lifecycle by introducing a timeout lifecycle event + handler, wiring it into workflow start, and validating via a new master integration test.

Changes:

Introduce WorkflowTimeoutLifecycleEvent / WorkflowTimeoutLifecycleEventHandler and add TIMEOUT to workflow lifecycle event types.
Publish workflow-timeout events on workflow start and add alert sending for workflow timeouts via WorkflowAlertManager → AlertDao.
Add a master integration test + YAML fixture to assert a workflow-timeout alert is persisted.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/alert/WorkflowAlertManager.java	Adds `sendWorkflowTimeoutAlert` entrypoint delegating to `AlertDao`.
dolphinscheduler-master/src/test/resources/it/start/workflow_with_workflow_timeout_alert.yaml	New IT fixture defining a workflow with `timeout` + `warningGroupId`.
dolphinscheduler-master/src/test/java/org/apache/dolphinscheduler/server/master/integration/cases/WorkflowStartTestCase.java	New IT case asserting workflow-timeout alert is created.
dolphinscheduler-master/src/test/java/org/apache/dolphinscheduler/server/master/integration/WorkflowOperator.java	Passes `warningGroupId` into manual trigger request; extends DTO.
dolphinscheduler-master/src/test/java/org/apache/dolphinscheduler/server/master/integration/Repository.java	Adds query helper for alerts by workflow instance id.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandler.java	New handler that sends workflow-timeout alerts.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/handler/WorkflowStartLifecycleEventHandler.java	Publishes workflow-timeout event when workflow starts and timeout is configured.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java	New delayed lifecycle event representing a workflow timeout.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/WorkflowLifecycleEventType.java	Adds `TIMEOUT` workflow lifecycle event type.
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/task/lifecycle/event/TaskTimeoutLifecycleEvent.java	Tightens timeout precondition to `> 0`.
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/AlertDao.java	Validates inputs and unifies timeout alert persistence helper method.
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AlertType.java	Updates enum comment for alert type codes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T02:30:49Z

...inscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java

+        long delayTime = System.currentTimeMillis() - workflowInstance.getRestartTime().getTime()
+                + TimeUnit.MINUTES.toMillis(timeout);


WorkflowTimeoutLifecycleEvent.of assumes workflowInstance.getRestartTime() is non-null and uses it to compute delayTime. In practice restartTime can be null (there are call sites that explicitly fall back to startTime when it's missing), which would cause an NPE and prevent timeout handling. Additionally, the current delay calculation adds the elapsed time since restartTime to the configured timeout, which schedules the event later than the configured timeout if there is any delay between setting restartTime and publishing the event. Consider falling back to startTime when restartTime is null and computing the remaining delay until (restart/start time + timeout) (clamping to 0 if already exceeded).

Suggested change

long delayTime = System.currentTimeMillis() - workflowInstance.getRestartTime().getTime()

+ TimeUnit.MINUTES.toMillis(timeout);

final long baseTimeMillis =

workflowInstance.getRestartTime() != null

? workflowInstance.getRestartTime().getTime()

: (workflowInstance.getStartTime() != null

? workflowInstance.getStartTime().getTime()

: 0L);

checkState(baseTimeMillis > 0,

"The workflow instance startTime and restartTime must not both be null when creating workflow timeout event.");

final long timeoutMillis = TimeUnit.MINUTES.toMillis(timeout);

final long deadlineMillis = baseTimeMillis + timeoutMillis;

final long delayTime = Math.max(0L, deadlineMillis - System.currentTimeMillis());

no need, restartTime is not null

Copilot · 2026-03-25T02:30:49Z

...er/src/test/java/org/apache/dolphinscheduler/server/master/integration/WorkflowOperator.java

                .dryRun(workflowTriggerDTO.getDryRun())
                .taskDependType(workflowTriggerDTO.getTaskDependType())
                .failureStrategy(workflowTriggerDTO.getFailureStrategy())
+                .warningGroupId(workflowTriggerDTO.getWarningGroupId())


WorkflowTriggerDTO now contains warningType, but manualTriggerWorkflow doesn't propagate it into WorkflowManualTriggerRequest (only warningGroupId is set). This makes the DTO field ineffective and can lead to confusion when writing tests that intend to control warningType. Either remove warningType from the DTO or pass it through to the request builder.

Suggested change

.warningGroupId(workflowTriggerDTO.getWarningGroupId())

.warningGroupId(workflowTriggerDTO.getWarningGroupId())

.warningType(workflowTriggerDTO.getWarningType())

sonarqubecloud · 2026-03-25T05:54:31Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

njnu-seafish · 2026-03-25T07:48:56Z

@ruanwenjun @SbloodyS Whenever you have time, I’d be grateful for your review. Thanks so much!

add workflow timeout

b12b82b

njnu-seafish requested review from SbloodyS, caishunfeng and ruanwenjun as code owners March 13, 2026 03:05

github-actions bot added the backend label Mar 13, 2026

github-actions bot assigned njnu-seafish Mar 13, 2026

njnu-seafish and others added 3 commits March 13, 2026 14:14

Merge branch 'dev' into Fix-17817-2

2b02024

add workflow timeout event and handle

6c30466

Merge branch 'dev' into Fix-17817-2

ac5373d

SbloodyS added the bug Something isn't working label Mar 13, 2026

SbloodyS added this to the 3.4.2 milestone Mar 13, 2026

github-advanced-security bot found potential problems Mar 13, 2026

View reviewed changes

SbloodyS requested a review from Copilot March 18, 2026 03:02

Copilot started reviewing on behalf of SbloodyS March 18, 2026 03:02 View session

Copilot AI reviewed Mar 18, 2026

View reviewed changes

ruanwenjun reviewed Mar 18, 2026

View reviewed changes

njnu-seafish and others added 4 commits March 19, 2026 18:51

Merge branch 'apache:dev' into Fix-17817-2

bc39a58

use restartTime

d31aa6e

use restartTime

1a18b36

add it test case for timeout

e4f43df

github-actions bot added the test label Mar 20, 2026

Merge branch 'dev' into Fix-17817-2

ed36525

njnu-seafish requested a review from ruanwenjun March 20, 2026 08:07

njnu-seafish and others added 2 commits March 23, 2026 09:26

Merge branch 'apache:dev' into Fix-17817-2

73832b8

add warningGroupId

65e9b5e

SbloodyS requested a review from Copilot March 25, 2026 02:24

Copilot started reviewing on behalf of SbloodyS March 25, 2026 02:24 View session

SbloodyS reviewed Mar 25, 2026

View reviewed changes

Copilot AI reviewed Mar 25, 2026

View reviewed changes

njnu-seafish and others added 2 commits March 25, 2026 10:45

Merge branch 'apache:dev' into Fix-17817-2

db34910

add newline

ca0c133

njnu-seafish requested a review from SbloodyS March 25, 2026 03:10

		long delayTime = System.currentTimeMillis() - workflowInstance.getStartTime().getTime()
		+ TimeUnit.MINUTES.toMillis(timeout);

	updateTime: 2024-08-12 00:00:00
	updateTime: 2024-08-12 00:00:00

-        long delayTime = System.currentTimeMillis() - workflowInstance.getRestartTime().getTime()
-                + TimeUnit.MINUTES.toMillis(timeout);
+        final long baseTimeMillis =
+                workflowInstance.getRestartTime() != null
+                        ? workflowInstance.getRestartTime().getTime()
+                        : (workflowInstance.getStartTime() != null
+                                ? workflowInstance.getStartTime().getTime()
+                                : 0L);
+        checkState(baseTimeMillis > 0,
+                "The workflow instance startTime and restartTime must not both be null when creating workflow timeout event.");
+        final long timeoutMillis = TimeUnit.MINUTES.toMillis(timeout);
+        final long deadlineMillis = baseTimeMillis + timeoutMillis;
+        final long delayTime = Math.max(0L, deadlineMillis - System.currentTimeMillis());

	.warningGroupId(workflowTriggerDTO.getWarningGroupId())
	.warningGroupId(workflowTriggerDTO.getWarningGroupId())
	.warningType(workflowTriggerDTO.getWarningType())

Conversation

njnu-seafish commented Mar 13, 2026

Was this PR generated or assisted by AI?

Purpose of the pull request

Brief change log

Verify this pull request

Pull Request Notice

Uh oh!

Check notice

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruanwenjun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njnu-seafish commented Mar 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Mar 25, 2026

Quality Gate failed

Uh oh!

njnu-seafish commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ruanwenjun left a comment •

edited

Loading